-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use collectionFormat multi for query params of repeated fields #902
Use collectionFormat multi for query params of repeated fields #902
Conversation
79316f1
to
5898668
Compare
Solves grpc-ecosystem#756 . Also formats protoc-gen-swagger/genswagger/template_test.go according to go fmt.
5898668
to
25f40c4
Compare
Looks like really great work! FYI, I think the way to fix the generate CI failure is to run
which will regenerate the examples. I'm really looking forward to this! |
Thanks @achew22 ! A docker run to generate these files will be great - I was having trouble figuring out how to get the right version of When I run the command you posted I get
I have |
That's very strange, this is the command run by CI when generating the diff that's failing. Are you sure you ran the command correctly? |
Ah, there is a problem with the mount path in Andrews link. Try this:
|
FYI the source of truth for this command is CONTRIBUTING.md |
(Andrews command is how we used to run it before we switched to go modules). |
Codecov Report
@@ Coverage Diff @@
## master #902 +/- ##
==========================================
+ Coverage 53.59% 53.61% +0.02%
==========================================
Files 39 39
Lines 3935 3937 +2
==========================================
+ Hits 2109 2111 +2
Misses 1629 1629
Partials 197 197
Continue to review full report at Codecov.
|
Note to self: tests are a bit flaky, we should consider increasing the wait for the server to start up. |
Thanks for your contribution! |
We should make a release with this new functionality |
I believe that this may have inadvertently broken spec...
This spits out the following error when I run
|
Thanks for your report @mmarod, could you raise an issue please? That is unfortunate. |
My immediate thought is that is happens when we have repeated fields in the body of a request instead of in the query parameters of the URL. Might just need a little extra logic. |
I raised #906 |
I have same trouble when do swagger spec validation... Look here |
Since that makes two of you that have spoken up I'm going to revert this until we can fix #906 at the same time. |
I've reverted the change in master and will make another release. |
…ecosystem#902) * Use collectionFormat multi for query params of repeated fields Fixes grpc-ecosystem#756 . Also formats protoc-gen-swagger/genswagger/template_test.go according to go fmt.
grpc-ecosystem#902)" This reverts commit 2b6cab6. It was found to have introduced a swagger spec violation, see grpc-ecosystem#906.
Solves #756 . Also formats
protoc-gen-swagger/genswagger/template_test.go
according togo fmt
.